Skip to content

fix: catch rejected promises from async event listeners (#136)#137

Merged
coji merged 2 commits intomainfrom
fix/async-listener-promise-handling
Mar 16, 2026
Merged

fix: catch rejected promises from async event listeners (#136)#137
coji merged 2 commits intomainfrom
fix/async-listener-promise-handling

Conversation

@coji
Copy link
Owner

@coji coji commented Mar 16, 2026 •
edited by coderabbitai bot
Loading

Summary

  • Detect when event listeners return a thenable and attach .catch() to forward rejections to onError (emit stays synchronous — no await)
  • Fix withLogPersistence() to use explicit void ... fire-and-forget instead of async listener whose Promise was silently dropped
  • Add tests: async rejection reaches onError, emit doesn't block on async listeners

Closes #136

Test plan

  • pnpm validate — 28/28 (301 tests)
  • pnpm --filter @coji/durably test:node:postgres — PostgreSQL tests

🤖 Generated with Claude Code

Summary by CodeRabbit

リリースノート

  • バグ修正

    • 非同期リスナーのエラーが正しくグローバルエラーハンドラーにルーティングされるようになりました
    • エラー処理ロジックを統一し、より一貫した例外処理を実装しました

  • パフォーマンス改善

    • ログ永続化をバックグラウンド処理に変更し、イベント発行がブロックされなくなりました

  • テスト

    • 非同期リスナーのエラー伝播とイベント発行の同期性を検証するテストを追加しました

@coji @claude
fix: catch rejected promises from async event listeners (#136)
f199526 
- Detect thenable returns from listeners and attach .catch() to forward
  rejections to onError (without awaiting — emit stays synchronous)
- Fix withLogPersistence() to use explicit fire-and-forget instead of
  async listener whose Promise was silently dropped

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 16, 2026

Deployment failed with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/mizoguchi-cojis-projects?upgradeToPro=build-rate-limit

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026 •
edited
Loading

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

ウォークスルー

このPRは、イベントリスナーが返すPromiseを検出し、拒否をグローバルエラーハンドラーに転送する仕組みを追加します。ログ永続化を待たないファイアアンドフォーゲット形式に変更し、エラー正規化ユーティリティを導入し、非同期リスナーのエラー処理テストを追加します。

変更内容

コホート / ファイル(s) サマリー
エラーハンドリング基盤
packages/durably/src/errors.ts		 未知の型のエラーをError型に正規化するtoError()ユーティリティ関数を追加。
イベントリスナー非同期対応
packages/durably/src/events.ts		 リスナーの戻り値がPromiseの場合、.catch()を使用して拒否をグローバルエラーハンドラーにルーティング。emitは同期のままで、Promiseは待たない。
エラーハンドリング正規化
packages/durably/src/job.ts		 Promise拒否時の手動エラー構築を、新しいtoError()ユーティリティで統一。
ログ永続化リファクタリング
packages/durably/src/plugins/log-persistence.ts		 ログ作成処理をファイアアンドフォーゲット形式に変更（asyncを削除し、awaitせず）。エラーはグローバルハンドラーを経由。
非同期リスナーエラーテスト
packages/durably/tests/shared/events.shared.ts		 非同期リスナーの拒否時にonErrorが呼ばれることを確認するテストと、emitが呼び出し側から見て同期的であることを確認するテストを追加。

推定コードレビュー労力

🎯 3 (中程度) | ⏱️ ~20 分

詩

🐰 Promise の行方を追いかけて、
静かに catch で拾い上げ、
エラーは onError へと舞い込む。
Fire-and-forget、気楽に、軽く、
非同期の道も安心に歩む 🌙

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR title directly matches the main objective: catching rejected promises from async event listeners.
Linked Issues check ✅ Passed Changes implement all coding requirements from #136: detect thenable returns, attach .catch() to forward rejections to onError, fix withLogPersistence() fire-and-forget pattern, and add tests.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue objectives; no extraneous modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/async-listener-promise-handling
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coji @claude
refactor: extract toError helper and deduplicate error handling
9506c99 
- Add toError(error: unknown): Error to errors.ts
- Use reportError local in emit() to eliminate duplicated guard block
- Replace inline ternary in job.ts with toError()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coji coji merged commit 049e8f6 into main Mar 16, 2026
3 of 5 checks passed
@coji coji deleted the fix/async-listener-promise-handling branch March 16, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: async event listeners silently drop Promises

1 participant

@coji